Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

USHIFT-5286: Update image mode document with bootc image builder (BIB) #4387

Merged
merged 1 commit into from
Jan 17, 2025
Merged

USHIFT-5286: Update image mode document with bootc image builder (BIB) #4387

merged 1 commit into from
Jan 17, 2025

Conversation

praveenkumar
Copy link
Contributor

Which issue(s) this PR addresses:

Closes #

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 7, 2025
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Jan 7, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ggiguash
Copy link
Contributor

ggiguash commented Jan 7, 2025

@praveenkumar ,
I'm not sure we want to support these scripts in the dev repository. I believe, we should instead update the https://github.com/openshift/microshift/blob/main/docs/contributor/image_mode.md documentation and refer users to it when necessary.

@praveenkumar praveenkumar force-pushed the update_image_mode branch 2 times, most recently from c9b2352 to b72ecde Compare January 7, 2025 15:40
@praveenkumar
Copy link
Contributor Author

praveenkumar commented Jan 7, 2025 •
edited
Loading

@praveenkumar , I'm not sure we want to support these scripts in the dev repository. I believe, we should instead update the https://github.com/openshift/microshift/blob/main/docs/contributor/image_mode.md documentation and refer users to it when necessary.

@ggiguash you mean those script should be part of docs/config instead as part of scripts directory? I kind of have a hidden agenda around those scripts to be part of microshift (location doesn't matter) to consume directly it to my other project (like cloning it and execute). BTW we can even have e2e test for BIB + deploying final iso/qcow2 generated using those if this we want in future or is it already tested somewhere else?

@praveenkumar praveenkumar mentioned this pull request Jan 8, 2025
Open
@ggiguash
Copy link
Contributor

ggiguash commented Jan 8, 2025

you mean those script should be part of docs/config instead as part of scripts directory?

Such scripts are intended for devenv setup and we have previous experience with build.sh I would not want to repeat. Users took it as a base for their procedures, but it was very contextual and not correct for all the cases.

For what I'd like to see is an augmentation of the image_mode.md page with this w/f, using the same style - instructions rather than a full script.

@praveenkumar
Copy link
Contributor Author

you mean those script should be part of docs/config instead as part of scripts directory?

Such scripts are intended for devenv setup and we have previous experience with build.sh I would not want to repeat. Users took it as a base for their procedures, but it was very contextual and not correct for all the cases.

For what I'd like to see is an augmentation of the image_mode.md page with this w/f, using the same style - instructions rather than a full script.

@ggiguash Understood, will move the script to our project and update the image mode doc with BIB info in this PR.

@praveenkumar praveenkumar changed the title [WIP] Add scripts to create iso using bootc-image-builder Docs: Update image mode document with bootc image builder (BIB) Jan 15, 2025
@praveenkumar praveenkumar changed the title Docs: Update image mode document with bootc image builder (BIB) NO-ISSUE: Docs: Update image mode document with bootc image builder (BIB) Jan 15, 2025
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 15, 2025
@openshift-ci-robot
Copy link

@praveenkumar: This pull request explicitly references no jira issue.

In response to this:

Which issue(s) this PR addresses:

Closes #

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@praveenkumar
Copy link
Contributor Author

@ggiguash Please have a look, I updated the docs and remove script part.

ggiguash
ggiguash reviewed Jan 16, 2025
View reviewed changes
docs/contributor/image_mode.md Outdated
Copy the `install.iso` file to the `/var/lib/libvirt/images` directory.

```bash
sudo cp -Z ./output/bootiso/install.iso /var/lib/libvirt/images
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rename the install.iso to ${VMNAME}.iso during the copy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ggiguash Sure

ggiguash
ggiguash reviewed Jan 16, 2025
View reviewed changes
docs/contributor/image_mode.md Outdated
the bootc image builder process.

```bash
cat > config.toml <<EOFCONF
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about using the config.tomp file for only embedding kickstart.
What's the main point here to deviate from the kickstart settings in previous sections?

P.S. I'm not decided on which approach is better, so I'm looking for justification to introduce config.toml notion in this document.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ggiguash Use of kickstart file is required because of providing the required disk layout to work for microshift lvm part otherwise we can directly use BiB on the generated bootc microshift image.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that kickstart is required. I'm asking if we need propagate kickstart via config.toml rather than from virt-install command line, like in previous section.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, we can use virt-install cli to pass the kickstart but we should mention or link to the doc that how it can also be embedded with iso when using BIB so that created ISO can be used on other hypervisor?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap, that would work. Let's add a note pointing to upstread docs here?

@ggiguash
Copy link
Contributor

ggiguash commented Jan 17, 2025 •
edited by openshift-ci bot
Loading

/retitle USHIFT-5286: Update image mode document with bootc image builder (BIB)

@openshift-ci openshift-ci bot changed the title NO-ISSUE: Docs: Update image mode document with bootc image builder (BIB) USHIFT-5286: Update image mode document with bootc image builder (BIB) Jan 17, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jan 17, 2025 •
edited by openshift-ci bot
Loading

@praveenkumar: This pull request references USHIFT-5286 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

In response to this:

Which issue(s) this PR addresses:

Closes #

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@ggiguash
Copy link
Contributor

/assign @ggiguash

@ggiguash
Copy link
Contributor

/unhold

@praveenkumar praveenkumar marked this pull request as ready for review January 17, 2025 08:57
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 17, 2025
@openshift-ci openshift-ci bot requested review from dhensel-rh and pmtk January 17, 2025 08:57
@ggiguash
Copy link
Contributor

Thank you, @praveenkumar !

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 17, 2025
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Jan 17, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ggiguash, praveenkumar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 17, 2025
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Jan 17, 2025

@praveenkumar: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit cf7ae1b into openshift:main Jan 17, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ggiguash ggiguash ggiguash left review comments

@dhensel-rh dhensel-rh Awaiting requested review from dhensel-rh

@pmtk pmtk Awaiting requested review from pmtk

Assignees

@ggiguash ggiguash

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@praveenkumar @ggiguash @openshift-ci-robot